-
Notifications
You must be signed in to change notification settings - Fork 296
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Markdown diff linter #879
Conversation
/hold |
3864c40
to
89feba7
Compare
/unhold |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many thanks for getting this underway @ivanvc. Question below about pipeline approach, want to cover that off first before we go to much further.
2d222a1
to
cffd6de
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - The make markdown-diff-lint
recipe works well for me locally.
One minor nit about dependency install below, thanks @ivanvc
Add Makefile target, and scripts to lint only the modified Markdown files, failing only if the violation is within the changed lines. Signed-off-by: Ivan Valdes <[email protected]>
cffd6de
to
954cf31
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work @ivanvc Thank you!
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ivanvc, jmhbnz, spzala The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
I had this issue open for a while but couldn't get back to it.
It adds a GitHub workflow(EDIT: we decided to make this a standalone script, so it can be used with prow) that links changed Markdown files and fails if the issue is within the changed lines.It uses NodeJS' markdownlint-cli2, which has improvements over markdownlint-cli. I introduced a configuration file,
.markdownlint-cli2.yaml
, which allows lines longer than 80 characters, as this rule would raise issues in every Markdown file we have in the repository.This should help with reviews, avoiding discussing Markdown formatting.
An example of a run at my fork: ivanvc#6.
Fixes #857.